This is an important function to review.

This is fine. What you should do next is update the friendly rendering to use the experimental api. It will simplify things.

We should render the datum hashes as well. See my other comment.

Changed from:

instance ToJSON HashableScriptData where
  toJSON = scriptDataToJsonDetailedSchema

to:

instance ToJSON HashableScriptData where
  toJSON hsd =
    object
      [ "bytes" .= (String $ Text.decodeUtf8 $ getOriginalScriptDataBytes hsd)
      , "json" .= scriptDataToJsonDetailedSchema hsd
      ]

No this is not that useful. We want the hash otherwise users will need to calculate the hash themselves.

Ah sorry I didn't understood that the first time. Did this change:

@@ -29,6 +28,6 @@ instance (L.EraTxOut ledgerera, L.EraGov ledgerera) => ToJSON (L.NewEpochState l
 instance ToJSON HashableScriptData where
   toJSON hsd =
     object
-      [ "bytes" .= String (Text.decodeUtf8 $ getOriginalScriptDataBytes hsd)
+      [ "hash" .= hashScriptDataBytes hsd
       , "json" .= scriptDataToJsonDetailedSchema hsd
       ]

This is an important choice to review.

Vide comment from cardano-api:

-- | Warning: Creating 'HashableScriptData' from a 'ScriptData' value pretty
-- much guarantees the original bytes used to create the 'ScriptData'
-- value will be different if we serialize `HashableScriptData` again.
-- Do not use this.
unsafeHashableScriptData :: ScriptData -> HashableScriptData
unsafeHashableScriptData sd = HashableScriptData (serialiseToCBOR sd) sd

I understand that it means that we can't be sure that deserialisation from JSON representation into HashableScriptData will generate the value with the same CBOR representation. (why?)

If that is the case here, we should prohibit from deserialising from JSON I think, for example like that:

instance TypeError (Text "Cannot deserialise HashableScriptData." :$$:
                    Text "Some good justification")
      => FromJSON HashableScriptData where
fromJSON = error "unreachable"

I understand that it means that we can't be sure that deserialisation from JSON representation into HashableScriptData will generate the value with the same CBOR representation.

Ledger's CBOR serialization is not guaranteed to round trip and this can change the hash. See IntersectMBO/cardano-ledger#2943 (comment)

However I don't see the issue here. scriptDataToJsonDetailedSchema does not call unsafeHashableScriptData. If we were rendering the hash (which we should) then we have to be careful and use hashScriptDataBytes.

@Jimbo4350 there's no issue yet. It's about making hidden assumptions clear. If someone writes naive FromJSON instance those two expressions could give different results:

hsd = undefined :: HashableScriptData

Right $ hashScriptDataBytes hsd

and

(hashScriptDataBytes . scriptDataFromJsonNoSchema) <$> (Aeson.eitherDecode $ Aeson.encode hsd)

My point is to make that non-roundtripping assumption clearer, or somehow expressed in the type system.

Vide comment from cardano-api:

-- | Warning: Creating 'HashableScriptData' from a 'ScriptData' value pretty
-- much guarantees the original bytes used to create the 'ScriptData'
-- value will be different if we serialize `HashableScriptData` again.
-- Do not use this.
unsafeHashableScriptData :: ScriptData -> HashableScriptData
unsafeHashableScriptData sd = HashableScriptData (serialiseToCBOR sd) sd

I understand that it means that we can't be sure that deserialisation from JSON representation into HashableScriptData will generate the value with the same CBOR representation. (why?)

If that is the case here, we should prohibit from deserialising from JSON I think, for example like that:

instance TypeError (Text "Cannot deserialise HashableScriptData." :$$:
                    Text "Some good justification")
      => FromJSON HashableScriptData where
fromJSON = error "unreachable"